Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always re-solve directory dependencies #6843

Merged
merged 19 commits into from
Oct 30, 2022

Conversation

adriangb
Copy link
Contributor

@adriangb adriangb commented Oct 20, 2022

Resolves #3783

This resolves an issue poetry lock --no-update does not recognize updates made to a path dependency because it treats it as immutable

@neersighted this works in practice on a real project, but I'm a bit at a loss as to where to add this. I think it should go in test_solver.py, but I think I'd have to re-create the dependencies manually by adding them to the repo, which seems tedious. Might need a bit of guidance there.

@dimbleby
Copy link
Contributor

cf #3783

@adriangb
Copy link
Contributor Author

@dimbleby thank you for linking to the related issues. Please let me know what you think this PR needs, in particular if we want any tests I would appreciate some guidance along the lines of "please write a test similar to this existing test in this test file" since I'm having a little bit of trouble figuring out what the testing strategy is for the project.

@radoering
Copy link
Member

A test may look similar to

@pytest.mark.parametrize("is_locked", [False, True])
def test_solver_does_not_update_ref_of_locked_vcs_package(

(except that the directory dependency is updated even if a prior version is locked).

@adriangb
Copy link
Contributor Author

@radoering test added, thanks for the pointer!

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After taking a closer look at the show command, I came to the conclusion that we don't want to re-solve path dependencies in any case. We only want to re-solve when refreshing the lock file but not when solving for a specific environment for show.

I think we have to try another approach: We shouldn't touch get_locked() but add path dependencies to use_latest when refreshing the lock file. If you want to try that approach, this should be the place to add packages with source type "directory" to use_latest:

ops = solver.solve(use_latest=[]).calculate_operations()

src/poetry/console/commands/show.py Outdated Show resolved Hide resolved
@adriangb
Copy link
Contributor Author

@radoering ok I think I got that approach to work, thanks for the feedback!

radoering
radoering previously approved these changes Oct 30, 2022
@radoering radoering enabled auto-merge (squash) October 30, 2022 14:41
@radoering radoering merged commit 2a70d67 into python-poetry:master Oct 30, 2022
@adriangb adriangb deleted the relock-path branch October 30, 2022 18:29
@adriangb adriangb restored the relock-path branch October 30, 2022 18:29
@adriangb adriangb deleted the relock-path branch October 30, 2022 18:29
@adriangb
Copy link
Contributor Author

@radoering thank you!

@skovhus
Copy link

skovhus commented Oct 30, 2022

Thanks for fixing this @adriangb!

@neersighted neersighted added area/solver Related to the dependency resolver kind/enhancement Not a bug or feature, but improves usability or performance impact/changelog Requires a changelog entry labels Nov 3, 2022
@neersighted neersighted added this to the 1.3 milestone Nov 3, 2022
@radoering radoering changed the title Always re-solve path dependencies Always re-solve directory dependencies Dec 2, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/solver Related to the dependency resolver impact/changelog Requires a changelog entry kind/enhancement Not a bug or feature, but improves usability or performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poetry lock --no-update does not reflect directory packages changes
5 participants